Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add tfport-data-link timeseries schema and SMF config adds. #6918

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add tfport-data-link timeseries schema and SMF config adds. #6918

wants to merge 4 commits into from

Conversation

zeeshanlakhani
Copy link
Collaborator

@zeeshanlakhani zeeshanlakhani commented Oct 22, 2024 •
edited
Loading

PR related to https://github.com/oxidecomputer/dendrite/pull/1033.

New timeseries (from sled-data-link) as we've added switch information
in the metadata.

Related:

zeeshanlakhani
zeeshanlakhani commented Oct 29, 2024
View reviewed changes
oximeter/oximeter/schema/tfport-data-link.toml
@@ -0,0 +1,132 @@
format_version = 1
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bnaecker this is a different timeseries (from sled-data-link) due to the switch information fields.

@zeeshanlakhani zeeshanlakhani marked this pull request as ready for review October 29, 2024 20:11
@zeeshanlakhani zeeshanlakhani self-assigned this Oct 30, 2024
@zeeshanlakhani zeeshanlakhani force-pushed the zl/tfportd-oximeter branch 3 times, most recently from 4c9a71d to 819eded Compare November 1, 2024 02:35
Nieuwejaar
Nieuwejaar reviewed Nov 1, 2024
View reviewed changes
Copy link
Contributor

@Nieuwejaar Nieuwejaar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just skimming this, it looks like some (all?) of this is in the other PR you just sent. Is that right?

@zeeshanlakhani
Copy link
Collaborator Author

Just skimming this, it looks like some (all?) of this is in the other PR you just sent. Is that right?

This shows the diff against #6793. It'd be good to get that one in first and I'll switch this PR against main. But, I still needed green on that one (and the associated dendrite one).

@zeeshanlakhani zeeshanlakhani mentioned this pull request Nov 1, 2024
Draft
@zeeshanlakhani
Copy link
Collaborator Author

So, yeah, @Nieuwejaar, it'd be good to get #6793 in first, and then this, but definitely want your take on the pkt_source updates here.

@zeeshanlakhani
Copy link
Collaborator Author

zeeshanlakhani commented Nov 1, 2024 •
edited
Loading

Update:

I'll make these changes on this PR:

  • host/port => dpd_host, dpd_port (for tfportd). Separately will do this for mgd later.
  • helper function for filling in sled properties.

@zeeshanlakhani
Copy link
Collaborator Author

Will push conflict fixes, etc, once things are up to date (doesn't impact review)

@zeeshanlakhani
Copy link
Collaborator Author

Rust issues stuck behind: #6914.

@zeeshanlakhani zeeshanlakhani force-pushed the zl/smf-sled-idents branch 4 times, most recently from 50648d2 to 05c181e Compare November 5, 2024 13:16
Base automatically changed from zl/smf-sled-idents to main November 7, 2024 00:03
@zeeshanlakhani
Copy link
Collaborator Author

@bnaecker lmk if you have any further questions.

@zeeshanlakhani
Copy link
Collaborator Author

ping to @bnaecker to chat about the separate time series piece here.

@zeeshanlakhani
Copy link
Collaborator Author

Pinging @bnaecker here for review/discussion (and on the CP piece) before making conflict updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnaecker bnaecker Awaiting requested review from bnaecker

@Nieuwejaar Nieuwejaar Awaiting requested review from Nieuwejaar

Assignees

@zeeshanlakhani zeeshanlakhani

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zeeshanlakhani @Nieuwejaar